Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support general chat #1297

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Support general chat #1297

wants to merge 16 commits into from

Conversation

PIG208
Copy link
Member

@PIG208 PIG208 commented Jan 22, 2025

@PIG208 PIG208 force-pushed the pr-general-chat branch 11 times, most recently from da55117 to b38c9bc Compare January 29, 2025 23:59
@PIG208 PIG208 marked this pull request as ready for review January 30, 2025 23:48
@PIG208
Copy link
Member Author

PIG208 commented Jan 30, 2025

I expect #1301 to be merged pretty soon (which may need some handling for general chat as it cannot be resolved), but the rest of the PR should be ready for review.

The PR is stacked atop #1301.

@PIG208 PIG208 added the maintainer review PR ready for review by Zulip maintainers label Feb 6, 2025
@PIG208 PIG208 requested a review from chrisbobbe February 6, 2025 18:56
@PIG208
Copy link
Member Author

PIG208 commented Feb 7, 2025

Will rebase and add screenshots shortly.

@PIG208 PIG208 removed the request for review from chrisbobbe February 7, 2025 21:35
@PIG208 PIG208 removed the maintainer review PR ready for review by Zulip maintainers label Feb 7, 2025
instead of inferring it from a TopicName

Signed-off-by: Zixuan James Li <[email protected]>
This helps imply why we don't have the check when displayName is null

Signed-off-by: Zixuan James Li <[email protected]>
@PIG208
Copy link
Member Author

PIG208 commented Feb 8, 2025

Screenshots:

Autocomplete
autocomplete filtered auto complete topic
Message list page
channel narrow empty compose topic narrow combined
Inbox; no resolve button
inbox lack of resolve button

The '#channel > topic' style strings are not supposed to be translated
into different languages as they are Zulip's language of expressing the
desintation, not something bound to the English language.

The string needs to be re-translated in other languages, as the
placeholder is now different.

See also:
  zulip#1148 (comment)

Signed-off-by: Zixuan James Li <[email protected]>
This will later be useful for checking emptiness if
realmEmptyTopicDisplayName is given.

Signed-off-by: Zixuan James Li <[email protected]>
This behavior is designed to replace how sending to an empty topic is
effectively sending to "(no topic)". The key difference being that
the `TopicName.apiValue` is actually empty, instead of being converted to
"(no topic)" with `_computeTextNormalized`.

Signed-off-by: Zixuan James Li <[email protected]>
Signed-off-by: Zixuan James Li <[email protected]>
Look for `allow_empty_topic_name` and `empty_topic_name` under "Feature
level 334" in the API Changelog to verify the affected routes:
  https://zulip.com/api/changelog

Fixes: zulip#1250

Signed-off-by: Zixuan James Li <[email protected]>
@PIG208 PIG208 requested a review from chrisbobbe February 8, 2025 01:38
@PIG208 PIG208 added the maintainer review PR ready for review by Zulip maintainers label Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer review PR ready for review by Zulip maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants